feat(qwp): recycle the symbol dictionary at a threshold to lift the cardinality cap - #91
Draft
jovfer wants to merge 77 commits into
Draft
feat(qwp): recycle the symbol dictionary at a threshold to lift the cardinality cap#91jovfer wants to merge 77 commits into
jovfer wants to merge 77 commits into
Conversation
Pull the lock/quarantine engine-construction block out of build() into LineSenderBuilder.constructEngineOnSlotLocked()/constructEngineOnSlot(), and expose a QwpWebSocketSender.EngineRebuildFactory seam that build() installs on the connected sender once connect() succeeds. Pure refactor, zero behavior change: build() keeps its wide logical-lock scope spanning the connect loop; the standalone constructEngineOnSlot() acquires the lock only around construction, for a later symbol-dictionary epoch rebuild to reuse the identical construct/quarantine code path.
constructEngineOnSlotLocked() now returns a small ConstructedEngine result (engine + whether construction itself quarantined the slot) instead of a bare CursorSendEngine. build() seeds its own quarantined local from that verdict, restoring the pre-refactor invariant that a construction-time quarantine counts toward the one quarantine per build() attempt the connect loop's retry guard allows. Without this, a construction-time quarantine followed by an UnreplayableSlotException from connect() would take a second quarantineTornSlot pass instead of the original close-and-rethrow. constructEngineOnSlot(), the public factory entry Task 5 consumes, keeps its 8-arg signature and CursorSendEngine return type: it just unwraps ConstructedEngine.engine and discards the quarantined verdict, since the recycle path latches terminal on connect failure rather than quarantining.
Adds three connect-string keys for the upcoming QWP symbol-dictionary recycle feature: symbol_dict_reset (on/off, default on), symbol_dict_reset_threshold (distinct-symbol count that triggers a recycle, default 100_000, bounded by QwpConstants.MAX_SYMBOL_DICTIONARY_SIZE), and symbol_dict_reset_max_wait_millis (upper bound on how long a triggered recycle waits for an opportunistic window before forcing, default 30_000, 0 means opportunistic-only). This is config plumbing only: the three values land on QwpWebSocketSender as resetEnabled/resetThresholdSymbols/resetMaxWaitMillis fields with @testonly getters, following the catch_up_cap_gap_min_escalation_window_millis knob end-to-end (ConfigSchema registry, builder methods with WS-transport guards, both connect-string parse paths, wsConfigSnapshotForTest). Actual recycle behavior is a follow-up task.
Adds Sender.resetSymbolDictionary(), an advisory request to start a fresh symbol-dictionary epoch (default no-op; QwpWebSocketSender overrides it). QwpWebSocketSender.armIfEligible() re-evaluates arming at the tail of resetTableBuffersAfterFlush() -- the shared exit point for the plain flush, split flush, and close-path callers -- so the recycle arms once symbol_dict_reset is enabled and either the global dictionary reaches symbol_dict_reset_threshold distinct entries or a caller requested a manual reset. Arming deliberately ignores deltaDictEnabled: a sender degraded to full self-sufficient frames still benefits from bounding dictionary growth, and a manual request is honoured regardless of mode. Covered by SymbolDictRecycleArmingTest: threshold crossing, symbol_dict_reset=off never arming, the manual advisory API (both immediate and mid-batch-deferred arming), the split-flush path sharing the same arming tail, arming in full-dict (degraded) mode, and the no-op default on a non-WebSocket sender.
testArmsInFullDictMode previously substituted the manual resetSymbolDictionary() advisory request for crossing symbol_dict_reset_threshold, which never touches globalSymbolDictionary and so left a future deltaDictEnabled-conditioned regression in threshold-based arming undetected. Rewrite it to construct the sender through the widest connect(List<Endpoint>, ...) overload, which accepts both a custom symbolDictResetThresholdSymbols and the fault-injecting CursorSendEngine, and genuinely cross the threshold (registering a, b, c) while the sender stays degraded to full self-sufficient frames. No manual reset call remains in the test.
- rollFsnEpochBase now throws IllegalStateException when cursorSendLoop is non-null: the loop's externalFsnBase is a construction-time snapshot, never updated on a live loop, so rolling with a loop attached would silently desync sender-level FSN accessors from loop-level FSN emission. - testPreRollTargetAnswersTrueAfterRoll rolled the same already-connected sender that produced fsn1, so its raw engine watermark never reset and the pre-fix comparison (ackedFsn() == fsn1 >= fsn1) was also true -- the test could not fail. Rebuilt around a fresh rolled sender/engine via the existing createRolledSender helper, matching tests 3/5/6. Tests 2, 4, and 7 also rolled an already-connected sender (now rejected by the new guard) and are rewritten the same way.
Implement the table() barrier hook and recycleForDictReset() swap: once the symbol-dictionary recycle is armed (Task 3) and the ring is proven drained, table() tears the cursor I/O loop and engine down, rolls the FSN epoch base (Task 4), replaces the producer's symbol dictionary, and rebuilds the engine via the Task 1 EngineRebuildFactory before reconnecting -- all synchronously inside a single call. A failed rebuild latches recycleFailure as a terminal state: every frame existed before the swap was already proven acked, so no data is at risk, but the sender that observed the torn-down engine/loop refuses further use. checkRecycleFailure() covers table() and the flush-family entry points (flush, flushAndGetSequence, drain, awaitAckedFsn) -- deliberately not close(), which must still be able to tear down a latched sender.
Fix round 1 from review: - maybeRecycleForDictReset() now refuses before any teardown when engineRebuildFactory is null (every public connect() overload leaves it unset -- only Sender.build() installs one) or the cursor engine isn't owned by this sender (setCursorEngine(engine, false)'s contract). Without this, a connect()-built sender with the default-on recycle feature armed (via resetSymbolDictionary() or a threshold crossing) would reach step 6, NPE against the null factory, and latch itself terminal for no reason. - Step 5 also resets lastCommitBoundaryFsn -- it held a raw old-epoch FSN that does not survive the roll. - checkRecycleFailure() now also guards sendRow(), closing the fluent-chain corner where a caller continues .symbol(...).atNow() against a currentTableBuffer selected before the latch, without an intervening table() call. - Step 6 rewires cursorEngine.setSlotLockReleaseListener(...) on the rebuilt engine, restoring the pool early-wakeup notification that bypassing setCursorEngine had dropped. - testPostRecycleSlotContents now asserts the post-recycle slot directory listing against the exact expected fresh-state file set, replacing a bare Files.exists(...) check that proved nothing (the outgoing engine had a same-named file too). - New testConnectBuiltSenderNeverRecyclesWithoutFactory covers both ways such a sender can arm (manual request, threshold crossing): neither may recycle, throw, or stop the sender from working.
Adds SymbolDictRecycleMemoryModeTest, the sf_dir-omitted counterpart of SymbolDictRecycleTest: threshold-triggered recycle at an empty backlog, a content oracle proving the epoch boundary loses (and duplicates) nothing acked, and the same recycle under initial_connect_retry=async. All three pass unmodified against the existing recycle swap -- zero production changes -- confirming the factory's slotPath == null arm, CursorSendEngine's file-less close, and the table() barrier are already mode-agnostic.
Fills the maybeBlockForStarvedReset() stub: when a symbol-dict recycle is armed but the ring is not yet drained, opportunistically waits (parked, awaitAckedFsn-shaped) up to symbol_dict_reset_max_wait_millis for the outstanding acks before giving up for this armed window. resetMaxWaitMillis<=0 disables the wait entirely; at most one blocking wait runs per armed window (starvationWaitDoneThisArm); an open deferred-commit group is never waited on, since the server withholds its acks by design until the closing commit lands and this producer thread is the only one that could ever send that commit -- blocking there would just run out the clock every time. A timeout increments the new symbolDictResetStarvationTimeouts counter and leaves the recycle armed so a later drained table() call can still fire it. Verified the deferred-commit guard is load-bearing by temporarily removing it and confirming the test fails (blocks the full deadline instead of returning immediately) before restoring it.
Javadoc for symbol_dict_reset_max_wait_millis (Sender.java builder method, and the DEFAULT_.../field comments in QwpWebSocketSender.java) said the knob controls how long the recycle waits "before forcing the rebuild" and that 0 means "never forces". That is backwards: nothing is ever forced through. On timeout the wait gives up, increments the starvation counter, logs a warning, and stays armed -- the dictionary threshold is the only actual backstop. Rewrite all three to state the real policy: once the armed window exceeds the knob, the NEXT table(...) call may block the calling thread for up to the knob's value waiting for the backlog to drain; 0 disables blocking entirely. WARN log wording "re-arming opportunistically" -> "staying armed": the arm is never consumed on a timeout, so nothing re-arms. Test fixes in SymbolDictRecycleStarvationTest: - testTimeoutLogsAndReArms: the "second table() must not re-block" probe was placed after a row had been queued (pendingRowCount==1), so it short-circuited on maybeRecycleForDictReset()'s precondition guard before ever reaching the wait -- vacuously true regardless of starvationWaitDoneThisArm. Moved the probe earlier, to a table() call with pendingRowCount==0, so it actually exercises the "at most one blocking wait per armed window" guard. - testBlocksThenRecyclesWhenAcksArrive: raised maxWaitMillis from 400 to 700ms (keeping the 150ms release delay) so the full recycle path (I/O loop join, engine close, rebuild, fresh handshake) has real headroom under the elapsedMs<maxWaitMillis assertion instead of ~250ms. - Both testBlocksThenRecyclesWhenAcksArrive and testLatchedErrorDuringWaitThrows now join their helper thread (releaser/poisoner) in a finally block, so an assertion failure can no longer leak a non-daemon thread. Re-ran SymbolDictRecycleStarvationTest (5/5) and the full SymbolDictRecycle* battery (27/27), all green.
Adds SymbolDictRecycleOutageTest covering two interleavings between the symbol-dictionary recycle swap and events outside the producer's own control: a real connection outage on its own stream (recycle triggers while the pre-recycle I/O thread is mid-reconnect against a killed server; step 2's close() joins it, step 7's fresh connect recovers once the endpoint accepts again), and a sibling orphan drainer mid-drain (the recycle only tears down the foreground sender's own cursor engine/I/O loop, leaving a concurrently-gated BackgroundDrainer untouched and able to complete afterward). The third scenario from the task brief -- OrphanScanner.isCandidateOrphan rejecting an empty slot directory -- turned out to already be pinned by OrphanScannerTest#testIsCandidateOrphanDirect and #testEmptySlotDirIsNotAnOrphan, so no new test was added for it. Test-only change; no production code touched.
SymbolDictRecycleCatchUpSkipTest pins that recycleForDictReset()'s step 7 reconnect never pays for a delta-dictionary catch-up frame: the rebuilt engine sits on a freshly-emptied slot, so the new loop's sentDictCount mirror seeds from PersistedSymbolDict.recoveredSize() == 0 and setWireBaselineWithCatchUp's gate stays false for the whole first post-recycle connection. The core scenario chains the negative and positive observations in one test so the zero count is provably a property, not a handler blind spot: after the recycle sends zero zero-table frames and tiles ids from 0, the handler force-drops the connection, and the resulting UNPLANNED reconnect does catch up -- bounded to exactly the new epoch's symbols, never replaying the retired epoch's. A follow-up symbol then ships with a delta start above 0, pinning that resetSymbolDictStateForNewConnection() on the plain-reconnect path preserves sentMaxSymbolId rather than folding the recycle's baseline reset into itself. A second test repeats the zero-catch-up observation under initial_connect_retry=async, where step 7's reconnect funnels through ensureConnected()'s ASYNC arm and the handshake completes on the I/O thread. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K2Hw6TfgX9sBB2L8oSdaWn
rev-t10 traced resetSymbolDictStateForNewConnection() to its single call site in ensureConnected(), which the unplanned I/O-thread reconnect (swapClient) never reaches -- so the old comment credited a function that does not even run on this path. sentMaxSymbolId survives the plain reconnect because nothing touches it there; only recycleForDictReset()'s step 5 ever zeroes the baseline. The class javadoc's failure-mode attribution carried the same imprecision and is reworded to match. The assertion itself was traced sound (symbolDeltaBaseline() -> encoder.beginMessage -> deltaStart) and is unchanged; comment-only diff. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K2Hw6TfgX9sBB2L8oSdaWn
Adds SymbolDictRecycleCrashWindowsTest, pinning what a restarted sender recovers if the process crashes at each of four points around QwpWebSocketSender.recycleForDictReset()'s 8-step symbol-dictionary recycle swap: - (a) before step 2 (the barrier that starts the swap): the pre-recycle epoch's slot holds a fully-acked batch on disk. Recovery must find that residue, recognize it as already acked (nothing to replay) and resume the SAME dictionary rather than starting fresh. Constructed by closing fast against a server that never acks (so the fully-drained unlink never fires) and then stamping the ack watermark directly to declare the batch acked retroactively, mirroring DeltaDictRecoveryTest#writeAckWatermark. - (b) between step 3 (fully-drained close of the old engine) and step 6 (rebuild): the slot is empty. Constructed by driving a real recycle to completion and then closing immediately, before any flush touches the freshly-rebuilt engine -- finishClose treats "nothing published yet" as fully drained too, so this unlinks everything step 6 just created, leaving the same empty state step 3 alone would have left. - (c) after step 7 (reconnect), before the new epoch's first flush: the slot holds a freshly-rebuilt engine's own state files but no data. Constructed by snapshotting the rebuilt slot's bytes before closing (there is no supported way to release just the slot's OS flock without running finishClose's unlink), closing for real so nothing leaks, then restoring the snapshot on top of the vacated directory. - (d) an ordinary mid-operation crash one epoch into the post-recycle steady state, to prove the epoch swap does not corrupt normal backlog recovery. Uses the same close-fast-against-a-non-acking-server idiom as RecoveryReplayTest, but only after the recycle's fresh connection is established. Arms (b) and (c) both replay nothing and both look "empty" at first glance, but they are not the same recoverable state and a restarted engine can tell them apart: (c)'s slot carries a manifest with collapsed boundaries alongside a same-based, zero-frame active segment, which SegmentRing.recover()'s chain-building accepts as a RECOVERED (if empty) chain, while (b)'s slot carries no engine state at all and recovers as EMPTY. wasRecoveredFromDisk() is the pinned, distinguishing observable between the two, asserted explicitly instead of writing two assertion-for-assertion duplicate tests. Every arm's oracle: the recovered sender keeps ingesting; the symbols it and its predecessor registered are exactly and correctly reconstructable from the wire (each fresh server handler rebuilds the per-connection delta dictionary); and no data frame is delivered more than the at-least-once contract allows (each handler counts data frames so a spurious re-send would show up as an unexpected count). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K2Hw6TfgX9sBB2L8oSdaWn
Applies review findings 1, 2, 3, 4, 6, 7, 8 and 10 from the task-11 review (findings 5, 9, 11 are deferred to the whole-branch review): - Arm (b) never actually pinned the "empty slot" disk image it exists to test -- it inferred emptiness from wasRecoveredFromDisk()==false on the successor instead of asserting the crashed sender's own slot dir. Adds an explicit listDir(slot) == [".lock", ".lock.pid"] assertion right after the crashed sender closes (symmetric to arm (c)'s pre-existing file-set assertion), plus a second one after the successor's own fully-drained close, proving the empty-slot state is stable rather than a one-shot coincidence. This also replaces arm (b)'s closing assertion, whose message previously credited the successor's "recovery" for cleaning up a stale manifest that the crashed sender's own close had already removed. - Arm (c) snapshotted the freshly-rebuilt slot before waiting for the manager worker's asynchronous hot-spare provisioning to settle, so a mid-provision snapshot could race-capture a zero-magic spare that recovery would hard-fail on. Adds a bounded poll (awaitExactFileSet) before the snapshot, reusing the same expected file list (now a shared FRESH_REBUILD_FILES constant) as the post-restore assertion so the two can never drift apart. - Arms (a) and (d) loosely asserted recoveredMaxSymbolId() >= 1, which a leaked epoch-0-plus-epoch-1 dictionary would also satisfy. Tightens both to the deterministic exact value (1L), keeping their explanatory messages as-is now that the assertion actually proves what the message claims. - Widens QwpWireTestUtils.tableCount to public (its sibling frame helpers already are) and deletes this suite's local copy plus its now-unnecessary justification javadoc. - Inlines the AckAllHandler bindings in arms (b) and (c) that were never read (dict()/dataFrameCount() were only meaningful on the "fresh" server's handler, not the "crashed" one). - Class javadoc: notes that arm (c)'s snapshot/restore does not cover the logical slot lock (lives outside the slot dir; sender.close() reclaims it, acquireLogical recreates it -- benign), and corrects three mechanism imprecisions the review traced: the "never published" fully-drained check lives in close(boolean), not finishClose; a fully-drained close does not remove .lock/.lock.pid; and arm (b)'s SegmentRing.recover() counterpart is the no-manifest fall-through to Recovery.empty(), not the manifest-present collapse branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K2Hw6TfgX9sBB2L8oSdaWn
A sender that degrades to full self-sufficient frames after a symbol-dict persistence fault (disableDeltaDict) used to stay degraded for the rest of its life -- deltaDictEnabled was set once at engine construction and never re-evaluated. The symbol-dictionary recycle already rebuilds the cursor engine from scratch on every swap; make that rebuild also re-derive deltaDictEnabled from the fresh engine instead of carrying the old one's verdict forward. If the underlying fault has cleared, the next recycle heals the sender back into delta mode; if it has not, the fresh engine degrades again on its own first append, the same ordinary catchable LineSenderException as any other persistence fault -- a degrade, never a latched recycleFailure terminal state. Promote the epoch and starvation-timeout counters from @testonly accessors to permanent public API: getSymbolDictEpochForTest() becomes getSymbolDictEpoch(), and getSymbolDictResetStarvationTimeoutsForTest() becomes getSymbolDictResetStarvationTimeouts(). Both counters already existed; this only changes their visibility and documents their thread-safety contract (producer-thread-written, so a read from another thread is an eventually-consistent snapshot). Add a third counter, getSymbolDictResetsPerformed(), incremented alongside the epoch inside recycleForDictReset() -- the two move together today but are defined and incremented independently, since a future change could roll the epoch by some path other than a completed recycle swap. Migrate every existing call site of the two renamed getters (7 test files, 48 + 15 occurrences) to the new public names. SymbolDictRecycleHealingTest covers all of this: a fault-then-heal recycle proves deltaDictEnabled and wire framing both return to proper delta encoding (the second post-recycle frame's delta starts where the first left off and carries only the newly-added symbol, the shape only delta mode produces); a fault-persists variant proves the fresh engine degrades again without escaping as a raw error or latching the sender terminal; and a two-recycle run asserts the three metrics getters track correctly in lockstep while the starvation counter stays untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K2Hw6TfgX9sBB2L8oSdaWn
Applies task-12-review.md findings Q1-Q6.
Q1: the three symbol-dictionary-recycle metrics fields (symbolDictEpoch,
symbolDictResetsPerformed, symbolDictResetStarvationTimeouts) become
volatile -- they are permanent public API now, and their obvious reader
is a monitoring thread on some other thread, unlike every other
symbol-dictionary-recycle field they were modelled on. A plain,
non-volatile long gives a cross-thread reader no visibility guarantee at
all under the JMM (a polling loop can legally observe 0 forever), whereas
the javadoc claimed an "eventually-consistent snapshot". Reworded the
three javadocs to state the guarantee volatile actually gives: each read
sees the latest write the producer thread completed, with no atomicity
across the three counters -- a concurrent reader can see the epoch
already advanced while resets-performed still reflects the prior value,
even though the producer thread writes them on adjacent lines.
Q3: getSymbolDictResetStarvationTimeouts()'s javadoc named
recycleForDictReset() as the writer by importing getSymbolDictEpoch()'s
caveat sentence verbatim; the starvation counter is actually written in
maybeBlockForStarvedReset(). Named that method directly instead.
Q4: getSymbolDictResetsPerformed() said swaps this sender has
"completed" without stating that the counter advances at step 5, before
the engine rebuild (step 6) and reconnect (step 7) -- so a recycle that
later latches recycleFailure at step 6/7 still counts. Made that
explicit, mirroring the precision getSymbolDictEpoch() already had.
Q2: SymbolDictRecycleHealingTest's healing test asserted
isDeltaDictEnabledForTest() right after the recycle and attributed the
result to the healed facade, but the same assertion holds unconditionally
-- a fresh engine's construction never touches mmap, so it reports true
whether or not the facade was healed (the persistent-fault sibling test
proves this directly). Reworded the message to state what that assertion
actually pins, and added the discriminating check: re-assert after the
first post-recycle flush, the fresh engine's first real append -- a
still-armed facade would degrade it there, so staying true is real
evidence of healing.
Q5: AckAllHandler never reset its ack sequence per connection, unlike
CapturingAckHandler right below it in the same file. A rebuilt engine
restarts its raw FSNs at 0 after a recycle, so the stale, unreset
sequence could satisfy awaitAckedFsn with an ack for a frame that was
never published on the new connection -- a weaker gate than it looks,
even though the specific sequences in these tests happened not to
collide. Reset nextSeq on every new connection, matching the sibling
handler and existing repo precedent for connection-aware ack sequencing.
Q6: both fault-injection tests caught LineSenderException with a comment
claiming parity with MmapFaultDegradesTest's guard, but never actually
asserted the message MmapFaultDegradesTest checks
("failed to persist symbol dictionary before publish") -- so the catch
could equally have swallowed an unrelated connection-level exception.
Added the same message assertion to all three catch sites (two in the
persistent-fault test, one in the healing test) so the comment's claim
now holds.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K2Hw6TfgX9sBB2L8oSdaWn
The symbol-dictionary cap error told callers to close the sender and build a new one, but gave no way to avoid hitting the cap in the first place. Append a sentence pointing at the automatic dictionary reset knobs (symbol_dict_reset, symbol_dict_reset_threshold) and the manual Sender.resetSymbolDictionary() escape hatch, so the error message matches the recycle feature this client now ships. Raise QwpConstants.MAX_SYMBOL_DICTIONARY_SIZE from 1_000_000 to 2_000_000 to mirror the server-side constant of the same name, which moved to 2_000_000 in questdb OSS commit 306062e243 (#7468). That commit is contained in release tag 10.0.0, the support floor for this client, so client <= server holds across the whole supported fleet. Update DeltaDictCeilingTest and GlobalSymbolDictionaryTest, which pinned the old 1,000,000 value and message text, to the new cap and add a case that drives the dictionary to the cap with automatic reset disabled (symbol_dict_reset=off) and a threshold configured at the cap, confirming the refusal still fires and still names the reset valve even on a sender that has it switched off. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K2Hw6TfgX9sBB2L8oSdaWn
The new cap-reached-while-armed test never calls sender.flush(): the 2M fill goes through the raw GlobalSymbolDictionary test accessor, and the one Sender-routed call throws inside symbol() before a row completes. armIfEligible() only runs from the tail of a completed flush(), so it never had a code path available to flip isResetArmed() to true regardless of whether symbol_dict_reset was on or off -- the two assertFalse(ws.isResetArmed()) checks passed identically either way and proved nothing about the knob under test. Drop both assertions and note in the test's javadoc that arming semantics are out of scope here and are pinned instead by SymbolDictRecycleArmingTest.testArmsAtThreshold, which drives real rows through flush() and is the idiom that actually exercises armIfEligible(). The test's brief-mandated assertion -- the cap refusal still fires, with the new reset-valve message, when reset is disabled -- is untouched and remains the load-bearing check here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K2Hw6TfgX9sBB2L8oSdaWn
recycleForDictReset() no longer latches the sender terminal when the
recycle's reconnect fails. Step 7 moves out of the latching try block:
steps 1-6 still latch recycleFailure (a half-swapped sender genuinely
cannot make progress), but a failed ensureConnected() logs a warning and
rethrows to the triggering caller without latching. By step 7 the swap has
committed, so the sender is coherent - connected == false, loop and client
already closed and nulled by ensureConnected's own catch, the fresh engine
attached, the step-5 epoch and swap counters correctly left incremented -
and the ordinary sendRow() -> ensureConnected() path retries the connect,
and only the connect, on the next send. Nothing re-runs a teardown step,
and nothing can fire a second swap meanwhile: the fresh dictionary sits
below the threshold, manualResetRequested was consumed at step 5, and
maybeRecycleForDictReset requires connected.
This removes a default-configuration brick. A sender built with no
reconnect_* knob resolves initialConnectMode to OFF, so step 7 is a
single-shot connect; a server restart or an LB blip across a drained,
armed sender therefore latched every later table()/flush() call forever,
including seconds later once the endpoint was back.
Deferring the connect to the next send exposed a second defect, which this
commit also fixes. ensureConnected() calls
resetSymbolDictStateForNewConnection(), which cleared
currentBatchMaxSymbolId unconditionally. That watermark is batch-scoped,
not connection-scoped - a flush ships exactly [sentMaxSymbolId+1 ..
currentBatchMaxSymbolId] - and clearing it was harmless only because
build() connects before the application can register a symbol. On the
deferred path symbol() runs first, so the clear made the next flush ship
an empty delta while its rows referenced symbol id 0: rows on the wire
pointing at ids the server never received. The reset now runs only from
the drained state the old code assumed (no pending rows, no row in
progress).
testDefaultConfigRecycleSurvivesFailedReconnect pins both. It drives a
default-config sender (no reconnect knobs), kills the listener at a
drained instant, asserts the recycle's throw reaches the caller, asserts
recycleFailure is not latched by ingesting successfully once the endpoint
returns on the same port, and asserts the recovered stream defines every
symbol its rows reference. Against the pre-fix code it fails on the latch;
with the latch fixed but the watermark clear restored it fails on the
empty dictionary.
Documentation and hygiene alongside:
- getTotalFramesReplayed/getTotalFramesSent/getTotalReconnectAttempts/
getTotalReconnectsSucceeded/getTotalServerErrors now state that they
read the live send loop and therefore restart at 0 on every recycle
("since the last recycle"), and point at the lifetime-scoped
getSymbolDictEpoch/getSymbolDictResetsPerformed for correlation.
- Sender.resetSymbolDictionary(), its QwpWebSocketSender override and
LineSenderBuilder.symbolDictReset() now say that the manual valve is a
permanent no-op while symbol_dict_reset is off, because armIfEligible
gates on that knob.
- armIfEligible's javadoc names both call sites; resetSymbolDictionary()
calls it too, and the stale single-call-site claim invited a wrong
inlining refactor.
- The builder's symbolDictReset default references
DEFAULT_SYMBOL_DICT_RESET_ENABLED instead of a hardcoded true.
- SymbolDictRecycleOutageTest joins its trigger thread in a finally so an
assert failure cannot leave a thread inside the sender, and records why
the revived server's handshake count stays >= 1 rather than == 1.
- SymbolDictRecycleCrashWindowsTest inlines an unread handler binding, and
SymbolDictRecycleHealingTest's AckAllHandler carries a warning that its
per-connection ack reset assumes every connection change is a recycle
and must not be copied into a plain-reconnect test.
Client suite: 3045 run, 0 failures, 0 errors, 2 skipped.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K2Hw6TfgX9sBB2L8oSdaWn
Review C1: recycleForDictReset step 3 re-acquired the slot flock without waiting for a deferred close to release it. When the SF worker is wedged in a syscall past SegmentManager's bounded join, CursorSendEngine.close() returns with the flock retained and isCloseCompleted() false, releasing both from the worker's exit path; the step-6 rebuild then threw SlotLockContentionException on the retained flock and latched the sender permanently terminal -- converting exactly the transient disk stall the deferred-close machinery exists to survive into a hard sender death on the default (SF + recycle-on) path. recycleForDictReset now mirrors close()'s deferred-close discipline: awaitDeferredEngineClose parks (awaitAckedFsn-shaped) until the deferred cleanup confirms the flock release, re-arming the shared flock-release retry driver each pass like isSlotLockReleased() does. Only exhausting the 30 s budget -- a genuinely dead worker -- latches terminal, and that path hands the still-locked engine to retainedEngine so a pool re-probe recovers the slot's capacity if the worker ever exits; close() no longer clobbers slotLockReleased to true while such an engine is pending. SymbolDictRecycleDeferredCloseTest pins both branches with a wedged- worker harness (red-proofed: without the await, the survival test dies with the exact SlotLockContentionException the review traced). Review Mo1: getAckedFsn and awaitAckedFsn snapshot cursorEngine into a local (the recycle transitions it non-null -> null -> non-null), and while it is null they report the durable watermark the recycle barrier proved (new lastRecycleDurableFsn) instead of collapsing to -1. Review Mo2: PooledSender forwards resetSymbolDictionary() to the live delegate instead of inheriting the interface's no-op default; pinned by a pooled-path test. Review Mo5 (verified: released 9.4.x servers cap the dictionary at 1M): document the pre-10.0.0 compatibility constraint on the 2M client cap at QwpConstants.MAX_SYMBOL_DICTIONARY_SIZE and symbolDictReset(boolean). Review mi7/mi8: step 6 refuses a rebuilt engine that recovered from disk (the empties-the-slot contract was breached) and resets slotLockReleased after a successful swap. Review mi3: recycle javadoc now says seven steps / steps 2-6, the resetArmed and resetSymbolDictionary docs match the code, and EngineRebuildFactory moves out of the field block. Review Mo4: SymbolDictRecycleFsnContinuityTest wraps every test in assertMemoryLeak, and LineSenderBuilderWebSocketTest pins the config boundaries: threshold == 2M accepted on both config paths, the symbol_dict_reset=on parse branch, the invalid-value message, and the three fluent-setter transport guards. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
[PR Coverage check]😍 pass : 251 / 271 (92.62%) file detail
|
getAckedFsn()/awaitAckedFsn() read cursorEngine, fsnEpochBase and lastRecycleDurableFsn from monitoring threads while the recycle reassigns all three on the producer thread. The sibling observability counters went volatile in an earlier round for exactly this reader; these three carry the same contract, so a monitor could observe a fresh engine with a stale epoch base and report an FSN dip to -1. Also document resetSymbolDictionary() as producer-thread-only: it mutates unsynchronized producer state, and the fire-and-forget javadoc framing invited cross-thread calls (notably via PooledSender).
Red first: the outage tests now assert the store-and-forward contract (producer never sees a transport error nor a reconnect budget after the initial connect) instead of pinning the step-7 foreground connect they used to. The production change lands in the next commit.
Review-round and finding identifiers resolve to nothing once the PR is squashed; describe the behaviour instead. Also drop a close() that the try-with-resources already performs.
The builder's rebuild factory is the half that forwards the live error handler into constructEngineOnSlot -> quarantineTornSlot, and that hand-off is only observable when a rebuild really has a slot to set aside. The new test plants SegmentSkipQuarantineTest's tainted slot (several never-acked segments, the oldest one's magic overwritten) into the empty slot the recycle's step-3 close just left behind, then lets the real factory run: the recycle still commits, and the handler installed after build() receives the DATA_LOSS SenderError naming where the bytes were set aside. The sender is built without an error handler at all, so a regression that forwarded the build-time handler instead of the live one leaves the notification nowhere -- verified by mutating the delegation to the handlerless overload, which fails the new assertion. Also align the test javadocs with the recycle step numbering and drop the last review-era names. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ru8dSnpZsgkg3uPkfwyGvp
A monitor thread parked between the accessor's two volatile reads across a real recycle observes the (old base, fresh engine) pair; the clamp against the durable watermark keeps it from reading -1. The deferred-close park witness now also samples both accessors while the engine is null. Both assertions go red with the clamp or the durable write removed.
A resume while the SF worker is still wedged used to recompute the deadline and park the producer for a fresh full budget on every table()/sendRow(). The deadline now lives in a field for the pending close: later entries probe the close and rethrow without parking, and a completed await resets it so the heal path's second close gets its own budget. Pinned by timing the retried call.
isSlotLockReleased() re-probing a retained outgoing engine latches true; the recycle cleared it only at the commit, so the flag stayed true across the rebuild and a possible heal round-trip while the rebuilt engine already held the flock. The write now follows the await; close() derives the flag itself when a rebuild fails, so a pool never loses the slot. The fresh dictionary is allocated before the rebuild so the commit block cannot throw and leave a flock-holding engine unassigned.
After a recycle the close-drain timeout printed raw engine FSNs while flushAndGetSequence() hands out epoch-rebased ones, so the printed target fed back into awaitAckedFsn() answered a different question. The WARN, the thrown message and the loop's two replay log lines now use the external scale, matching the SenderError spans. Pinned after a real recycle.
symbol_dict_reset_threshold now stops at half the protocol cap, the re-arm floor's own ceiling: arming happens at a flush tail and the swap at the next drained table(), so a threshold at the cap tripped the cap error before any recycle could run. symbol_dict_reset_max_wait_millis rejects values whose nanosecond conversion overflows, which used to invert the knob into never blocking.
symbol_dict_reset_threshold now caps at half of MAX_SYMBOL_DICTIONARY_SIZE, so the fixture that configured it at the old full-cap value tripped the builder's own validator instead of reaching the dictionary-fill assertions it exists to cover. Point the fixture at the threshold's own new cap and reword the javadoc to name the two caps separately.
A recycle's reconnect fires RECONNECTED with no DISCONNECTED before it (pinned); a rebuild-time quarantine is delivered synchronously on the producer thread like the build-time one; a fresh loop on an engine recovered at build() seeds its mirror from the recovered dictionary, not from zero. setEngineRebuildFactory is documented and refuses a closed sender; getTotalAcks and getTotalBackpressureStalls say they restart on every recycle like their siblings.
The epoch counter and the completed-swaps counter were incremented on adjacent lines by the only code path that touches either, so they could never differ; the getter and the comments explaining a divergence the code cannot produce are gone. The epoch is the lifetime-scoped counter the per-recycle getters point at.
getSymbolDictEpoch()'s javadoc still said "three" symbol-dictionary- recycle counters. Only two remain after the swap counter was dropped: the epoch itself and the starvation-timeout counter.
Step 6 of recycleForDictReset()'s javadoc still said the epoch counter and a "completed-swap counter" both advanced. Only the epoch counter advances now. Fixed the same stale plural in three sibling spots: completeRecycleRebuild()'s summary, the swap's parenthetical (dictionary, counters, epoch), and recycleFailure's javadoc.
The interrupted-recycle test asserts the abandon branch instead of gating on it; the step-7 fault test no longer calls its own red-proof untested; the starvation suite carries timeouts and wall-clock margins that survive a busy runner; the crash-window suite quotes the recycle's actual seven steps; planning-document references are gone from test javadocs; the two FSN seams follow the ForTesting convention; one licence header matches its siblings.
SymbolDictRecycleFsnContinuityTest's class javadoc still named Task 5 as the source of the engine-rebuild behavior it pins. Reworded to describe the recycle itself, matching the planning- reference cleanup already done across the rest of this suite.
The full 3068-test client suite failed once at the hard assertion Step 1 of this task added: under load, an interrupted producer's loop close can complete normally instead of throwing, because CountDownLatch.await() can observe the shutdown latch already at zero by the time it re-checks after the interrupted exception, so close() proceeds instead of failing loud. The class alone (13/13) and a dedicated 20-iteration run both missed this race. The recovery half (never terminal, finishes the recycle on later sends) now runs unconditionally regardless of whether the loop close threw. The abandon-specific assertion is gated behind Assume.assumeTrue: a lost race reports the test as SKIPPED with an explanatory message instead of silently passing or intermittently failing. The re-asserted-interrupt-flag assertion still runs whenever the throw happened. Javadoc and inline comments updated to describe both outcomes instead of claiming the throw is deterministic.
The abandon WARN in drainOnClose now prints epoch-rebased FSNs like the timeout path does, instead of the raw ring scale. The epoch javadoc's atomicity caveat is now stated once instead of twice. The rebuild-factory setter's javadoc no longer claims that null cancels a recycle that is already armed or pending; it only stops future arming. The arming javadoc names the epoch counter instead of a nonexistent resets counter.
# Conflicts: # core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java # core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/BackgroundDrainer.java # core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/CursorWebSocketSendLoop.java
The merge conflict resolution had retyped the pre-existing connect(..., String authorizationHeader, ..., maxFrameRejections, poisonMinEscalationWindowMillis, catchUpCapGapMinEscalationWindowMillis) overload in place by appending the three symbol-dict-reset params, instead of adding a new overload alongside it. That breaks binary compatibility for callers compiled against the original 23-arg signature (NoSuchMethodError), which is exactly what ExportedApiCompatibilityTest.testPreBranchQwpWebSocketSenderConnectOverloadsStillLink caught. Restore the original 23-arg overload (forwarding to connectWithCredentialSupplier with the DEFAULT_SYMBOL_DICT_RESET_* constants) and keep the wider 26-arg overload as a separate, additional overload for callers that need to configure the recycle knobs explicitly on the String-credential path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
DEFAULT_SYMBOL_DICT_RESET_MAX_WAIT_MILLIS drops from 30_000 to 0: the recycle now runs only when a table() call finds the backlog already drained, and never blocks the producing thread. A positive value stays available as an explicit opt-in with unchanged semantics (one bounded wait per armed window). The builder javadoc states the trade plainly and names the monitoring signal for a recycle that never finds its drained instant: isResetArmed() staying true while getSymbolDictEpoch() does not advance. This restores the reconnect-reset design's posture (spec 13 s11.4): the bounded wait on the hot path was plan-added and its default was never signed off. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
maybeBlockForStarvedReset() and awaitDeferredEngineClose() park with LockSupport.parkNanos, which returns immediately while the caller's interrupt flag is set -- the wait degenerated into a busy spin, and a flag that survived to the drained exit made the swap's loop-close join throw, manufacturing a CLOSE_LOOP abandon out of a successful wait. Both loops now clear the flag per park iteration, restore it on the timeout/throw exits, and swallow it on the success exit, the same policy CursorSendEngine's flock-release retry driver uses. Two tests pin the policy: the drained exit must complete the recycle with no abandon and no leftover flag; the timeout exit must restore the flag after waiting out its full budget. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
awaitDeferredEngineClose() only restored a cleared interrupt flag inline, right before its own deadline throw. A stray RuntimeException or Error from isCloseCompleted() or ensureFlockReleaseRetryScheduled() after the flag was cleared would propagate around that inline restore and silently drop it -- the asymmetry maybeBlockForStarvedReset()'s wrap-the-whole-loop shape was designed to avoid. Wrap the loop in the same try/catch(Error)/catch(RuntimeException) shape as its sibling, restoring in both catch blocks and dropping the now-redundant inline restore before the deadline throw (that throw is a LineSenderException, itself a RuntimeException, so the catch below already covers it) -- every throw exit now restores exactly once, the completed exit still swallows. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
An abandoned recycle's CLOSE_LOOP resume used to drop sentMaxSymbolId with the dead loop's mirror, so in delta mode the next flush re-encoded the whole dictionary as one delta -- and a dictionary past the server batch cap could never flush again (the chunk fallback is full-dict only). The resume now re-registers [0..sentMaxSymbolId] as deferred dictionary chunks plus their closing commit on the ring; the fresh loop replays them ahead of any data frame, so the baseline survives. Any publish failure degrades to the old baseline drop, and the catch closes the chunks' deferred-commit debt itself: a commit failure is not covered by publishDictionaryChunks' internal orphan handling, and an open group would clamp ackedFsn for the connection's life. The publishDictionaryChunks assert now admits the delta-mode caller whose range the write-ahead persist made durable, and still rejects a delta-mode caller in any other state; memory mode (no persisted dict) and the degraded delta-to-full state stay accepted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
The CLOSE_LOOP resume's re-registration Error arm dropped the baseline and rethrew on the assumption that publishDictionaryChunks always closes its own chunk debt before an Error escapes it. That holds only for an Error thrown INSIDE publishDictionaryChunks. An Error from the resumeCommitFaultForTesting seam or from sendCommitMessage() lands in this arm AFTER publishDictionaryChunks already returned successfully, having published chunks and set hasDeferredMessages -- so the debt was left open, and an open deferred group clamps ackedFsn for the connection's whole life. The Error arm now closes that debt the same way the Throwable arm does: guarded by hasDeferredMessages, via the best-effort commitOrphanedDictionaryChunks(e), before dropping the baseline and rethrowing. The Error itself is never swallowed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
A fault injected between the resume's chunk publish and its commit must degrade to the shipped behaviour (baseline dropped, resume state cleared, next send works) and must leave no open deferred-commit group behind. The test pins both, and documents explicitly that the fallback does not heal the over-cap population -- it only refuses to wedge. A second test pins the Error arm: the Error propagates, yet the debt is closed and the baseline dropped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
A process death between the CLOSE_LOOP resume and the reconnect leaves the re-registration chunks, their commit, and the data frame on the SF ring. A fresh sender on the same slot replays them whole against a server that models the real deferred-ack contract (no ack for deferred frames until the group's commit): the drain completes and the replayed dictionary covers the old ids ahead of the data frames. The test also asserts hasDeferredMessagesForTesting()==false synchronously right after the resume runs, before the crash-simulated close. This is necessary, not decorative: the crash-replay outcome (drain + dictionary) alone cannot pin whether the resume itself closed its own deferred-commit debt, because two independent, pre-existing mechanisms backstop it regardless -- close()'s own redundant "!deferCommit && hasDeferredMessages" safety net (which still runs under close_flush_timeout_millis=0, since that only gates the drain-wait, not the local flush/commit/seal steps) and the recovered slot's own reconnect dictionary catch-up (CursorWebSocketSendLoop's sentDictCount/hasReplayDictionaryDependency mirror, seeded from the persisted .symbol-dict on every delta-mode recovery). Verified by mutation: deleting resumeRecycleIfPending's sendCommitMessage() call leaves drain()/dict() green, but the synchronous check catches it immediately. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
The crash-replay test's javadoc claimed the deferred-ack server "acks everything only because the commit frame is there to close it" -- implying the drain + dict-order outcome isolates the resume's own commit. It does not: close()'s own redundant commit-closing safety net and the recovered slot's reconnect dictionary catch-up both produce the same outcome even with the resume's commit missing entirely, as the inline comment beside the synchronous assertion already states. Rewrite the javadoc to say plainly what drain + dict-order actually proves (SF-ring persistence and in-order replay against a strict deferred-ack-withholding server) and name the synchronous hasDeferredMessagesForTesting() assertion as what isolates the resume's own commit. Also: update the suite's class-level javadoc from "four points" to "five points" with an (e) entry for the new test, and document DeferAwareCaptureHandler's single-connection assumption explicitly (no per-connection dict reset, so phase 2 must never reconnect against it). Documentation only; no test logic changed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
resetSymbolDictionary(), the dictionary-cap error message, and armIfEligible() now all say the same thing in one sentence each: the recycle runs at a table() call that finds the backlog acknowledged, and table() is the only trigger point. Sender.table() documents table-per-row as the row API's contract, so no per-row branch is added to the hot path for callers who hold one table open forever. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
The recycle swap seeds the fresh I/O loop with markEverConnected(), so an endpoint-policy rejection (401) on the post-recycle handshake stays transient: the sender keeps buffering and the rows land once the endpoint relents. Deleting the seed makes this test observe the latched terminal, which is exactly the data-loss regression it guards. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
Two real recycles with different per-epoch frame counts assert the exact base after each swap: (L1+1) after the first, (L1+1)+(L2+1) after the second, and the next published FSN lands exactly on the accumulated base. A base that is re-assigned instead of accumulated passes every monotonicity check in the existing suite and fails only these equalities -- the second recycle is the discriminator. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
- Drop core/.factorypath from the index and ignore it alongside its .classpath/.project siblings. - Make resetArmed volatile: isResetArmed() is a documented monitoring surface, matching the file's other volatile monitoring fields. - Document that getSymbolDictResetStarvationTimeouts() counts only the opt-in bounded wait, so it stays structurally 0 at the default symbol_dict_reset_max_wait_millis=0 -- 0 does not mean "no starvation". - Add the deferral caveat to both public entry points of the symbol_dict_reset knob (Sender#symbolDictReset and DEFAULT_SYMBOL_DICT_RESET_ENABLED): the recycle runs at a table() call that finds the backlog acknowledged, so it may be deferred indefinitely under sustained load. - Explain why the CLOSE_LOOP resume's baseline drop stays safe now that chunks are ringed before the drop, in both failure arms. - Rework publishDictionaryChunks' All-or-nothing javadoc for its two callers (the full-dict fallback and the delta-mode resume). - Snapshot the volatile serverMaxBatchSize cap once in the CLOSE_LOOP resume, matching the file's snapshot-once convention elsewhere. - Reword armIfEligible's javadoc: arming is "triggered" (not "consumed") from table(), and note a pending resume can still complete a recycle from a later flush()-driven ensureConnected(). - Restructure the resetMaxWaitMillis field comment to lead with the 0 default (opportunistic-only) before describing the opt-in bounded wait. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The WebSocket sender no longer has a hard symbol-cardinality stop. When its global symbol dictionary reaches a configurable threshold (
symbol_dict_reset_threshold, default 100 000), the sender recycles its send stack at the next provably-safe instant — all published frames acknowledged, no row in progress, no open transaction: it closes the send loop, retires the store-and-forward slot with the existing fully-drained close (emptying it), starts a fresh dictionary epoch on the same slot, rebuilds the engine through the same code pathbuild()uses, and reconnects. The server needs no change: supported servers (10.x+) already clear per-connection dictionary state on disconnect, and this branch pins that behavior as a wire contract. Zero wire change, zero on-disk format change; all production code is client-side (~1 500 net production lines, plus ~6 500 lines of tests).User-visible surface
Three connect-string keys / builder methods —
symbol_dict_reset(default on),symbol_dict_reset_threshold,symbol_dict_reset_max_wait_millis(bounded blocking wait for a starved reset; default0= opportunistic-only, a positive value opts the producer into blocking); an advisorySender.resetSymbolDictionary()(no-op on non-QWP transports; producer-thread-only, andPooledSenderforwards it to the live delegate); two observability getters onQwpWebSocketSender(getSymbolDictEpoch,getSymbolDictResetStarvationTimeouts). Externally visible frame sequence numbers stay strictly monotone across recycles via an epoch-base offset applied on every public surface (flush sequence, ack watermark, drain, progress stream, error spans). The client-side dictionary cap rises 1M → 2M (now equal to the server cap, with an OSS ordering test), and the cap error now points at the reset valve. A recycle also naturally heals a mid-life full-dict degrade (the fresh engine re-derives delta capability). Senders built via the publicconnect(...)overloads (no rebuild factory) and senders that don't own their engine never recycle and never arm, soisResetArmed()cannot read true against a permanently-zero resets counter.Release-note items:
CursorWebSocketSendLooppublic ctor and theQwpWebSocketSendermasterconnect(...)overload gained parameters in place.getTotalFramesSent,getTotalFramesReplayed,getTotalReconnectAttempts,getTotalReconnectsSucceeded,getTotalServerErrors, …) read the live cursor loop, which the recycle rebuilds, so they restart at 0 on every recycle — they are scoped "since the last recycle". Long-lived monitors that differenced them as lifetime counters should track the lifetime-scoped epoch counter (getSymbolDictEpoch) alongside, or treat a negative delta as a recycle boundary.getAckedFsn()'s contract is restated on theSenderinterface: on a live sender it never collapses back to −1 and keeps reporting the last durable watermark across a recycle.markEverConnected()is a new public method onQwpWebSocketSender(loop-side seeding of the sender-lifetime ever-connected flag).symbol_dict_reset_thresholdaccepts 1..1,000,000 (half the protocol cap, the re-arm floor's own ceiling);symbol_dict_reset_max_wait_millisrejects values whose nanosecond conversion overflows.getSymbolDictResetsPerformed()is gone — it could never differ fromgetSymbolDictEpoch().Recycle robustness
The through-line: the recycle prefers abandoning and resuming over latching or re-arming.
ensureConnected()latcheshasInitialConnectRunon its first completion and routes every later entry through the ASYNC branch — the loop is built and started synchronously, the socket connect happens on the I/O thread with indefinite retry, and the producer keeps buffering into the fresh epoch's slot. A default (OFF) or SYNC store-and-forward sender therefore never sees a transport error or a reconnect budget past its initial connect, matching the steady-state store-and-forward contract. The rebuilt loop is seeded with the sender-lifetime ever-connected flag (markEverConnected, read after the outgoing loop'sclose()has joined the I/O thread), sowasEverConnected()stays sticky and endpoint-policy failures keep their post-init retry classification across the swap.L·ln(L/(L−T))rows forever. Each swap raises the re-arm bar tomax(threshold, 2 × size-at-swap), capped at half the protocol cap so genuinely unbounded-cardinality producers keep recycling. No new config key; the default threshold is unchanged. Red-proofed: without the floor a bounded 6-symbol set recycles 4×, with it once. The floor arithmetic itself is pinned (doubling per swap, organic re-arm refused below it, advisory request bypasses it) — the integration suites' bounded symbol sets rely on that bypass to keep recycling.SegmentManager's bounded join (a stalled disk/NFS — exactly what the deferred-close machinery exists to survive).awaitDeferredEngineCloseparks until the worker's exit path confirms the release (30 s budget) instead of letting the rebuild throwSlotLockContentionException.SymbolDictRecycleDeferredCloseTestpins both branches with a wedged-worker harness and a positive parked witness; red-proofed against the pre-fix code (dies with the exactSlotLockContentionExceptiontrace). The same protocol closes an engine the rebuild cannot keep (recovered fully-acked leftovers): a wedged worker there retains the engine and resumes on the next send instead of colliding with the next rebuild, andclose()keeps reporting the flock held until the worker exits; pinned with a wedged recovered engine both riding the stall out and exhausting the await. Step 3 detaches the outgoing engine's slot-lock listener before closing it, so a release completing after the swap cannot mark the rebuilt engine's flock released; a recycle-retained engine reaches pools through theisSlotLockReleased()re-probe. The await budget is spent once per pending close: a resume while the worker is still wedged probes and rethrows without parking (pinned: the retriedtable()returns in well under the budget).CLOSE_LOOP/REBUILD) that the next send completes. The terminal latch survives for exactly one case: a rebuilt engine that recovered unacknowledged frames, which proves the outgoing close's fully-drained contract was breached. Benign recovered fully-acked leftovers (a transiently failed segment unlink) heal by closing the recovered engine — which retries the unlink by design — and rebuilding.Errorpasses through the recycle machinery unwrapped and unlatched. Suites pin the interrupt, rebuild-fault, await-exhaustion, heal, and breach paths, each red-proofed.rebuild(SenderErrorHandler)), not the builder's field captured at build time, sosetErrorHandler()afterbuild()is honoured when a recycle rebuild sets a torn slot aside; the builder's factory also snapshotssf_dir/sender id at build time.@TestOnlyfault hook drives a real failed reconnect: one test proves ids registered mid-window survive into the flushed delta (fails with theresetSymbolDictStateForNewConnectionguard reverted to the unconditional clear), the other proves the failure leaves the sender usable (fails withrecycleFailurelatched in the reconnect catch). Both counterfactuals re-run red against the final tree.close()'s drain guard is split by true dependency (flush/commit/seal need only the engine;checkUnsurfacedError/drainOnCloseneed the loop). Staged rows cannot coexist with a null loop —sendRow()callsensureConnected()before a row commits and every failure path rolls the in-progress row back — so this is hardening that keeps the guard honest for any future state, not a fix for a reachable row drop.getAckedFsn()/awaitAckedFsn()snapshotcursorEngine/cursorSendLoopinto locals and, while the engine is null (mid-swap), report the durable watermark the recycle barrier proved instead of collapsing to −1; the epoch base is read before the engine so a torn pair can never fabricate an FSN above anything published;cursorEngine,fsnEpochBase,lastRecycleDurableFsnand the ever-connected flag are volatile for monitoring readers; thegetAckedFsnjavadocs state the real post-recycle sentinel behavior, including the HTTP/TCP/UDP always-−1 caveat. The torn (old base, fresh engine) clamp is pinned by a test that parks a monitor thread between the accessor's two reads across a real recycle; the null-engine window is asserted from inside the deferred-close park witness — both go red with the clamp or the durable write removed.SymbolDictRecycleHealingTestpins that a degraded sender heals back into delta mode once the fault clears, and that a persistent fault degrades the fresh engine again as a normal, catchable error.drainOnClose's two WARNs (the abandon-uncommitted-frames one and the timeout one) and its thrown message, plus the I/O loop's two replay log lines, report epoch-rebased FSNs — the same scaleflushAndGetSequence()and theSenderErrorspans use (pinned after a real recycle).RECONNECTED/FAILED_OVERwith noDISCONNECTED(now onSenderConnectionEvent.KindandSenderConnectionListener, and pinned); a rebuild-time quarantine is delivered synchronously on the producer thread like the build-time one (SenderErrorHandler);setEngineRebuildFactoryis documented and refuses a closed sender.setEngineRebuildFactory(null)stops future arming but never cancels an armed or pending recycle, and its javadoc now says so.This update moves the recycle's default posture to fully opportunistic and closes two resume gaps found under interrupt and fault injection.
symbol_dict_reset_max_wait_millisnow defaults to0: a recycle only ever runs when atable()call finds the backlog already drained, and the producer is never blocked unless a positive value opts it in — restoring the reconnect-reset design's original posture, which the earlier default had drifted from. The two park loops a recycle can enter (maybeBlockForStarvedReset()'s starvation wait,awaitDeferredEngineClose()'s deferred-close wait) now share one interrupt policy — clear the flag per park iteration, restore it on every timeout/throw exit, swallow it on success — after a carried interrupt flag was found manufacturing a falseCLOSE_LOOPabandon out of an otherwise successful wait. ACLOSE_LOOPresume now re-registers the delta baseline it used to drop, republishing it as deferred dictionary chunks the fresh loop replays ahead of any data frame, and closes the outstanding chunk-commit debt on both the resume'sThrowableandErrorpaths, so an interrupted or faulted recycle converges the same way a clean one does.resetSymbolDictionary(), the dictionary-cap error, andarmIfEligible()'s javadoc now state the trigger point in one sentence each:table(...)is the only place a recycle ever runs. Eight new tests pin these: the interrupt policy under both park loops, the CLOSE_LOOP re-registration and its fallback/debt-close paths (including a crash-replay run proving the abandoned resume group survives a restart), a post-recycle endpoint-policy-rejection retry, and FSN epoch-base accumulation across two real recycles.Design decisions
symbol_dict_reset_max_wait_millis=0): the recycle runs when atable()call finds the backlog drained, per the reconnect-reset design's posture — a bounded wait onthe hot ingestion path is not a default. A positive value is an explicit opt-in that blocks the
producing thread at most once per armed window. The signal that a deployment never finds its drained
instant is
isResetArmed()staying true whilegetSymbolDictEpoch()does not advance; the plannedescalation for that population is an asynchronous handover of the dictionary swap to the I/O loop,
not a longer wait.
close()'s stall class, not always its blocking behavior. The swap'sloop-close wait is literally
close()'s own budget (CursorWebSocketSendLoop.close()'s 30 sshutdown-await) — the same method a plain
close()calls. The deferred-engine-close wait is aseparate, dedicated budget (
RECYCLE_DEFERRED_CLOSE_MAX_WAIT_MILLIS, also 30 s) sized to survive thesame disk/NFS stall class
SegmentManager's bounded worker join is built to survive; unlike a plainclose(), which spends only its own join budget and returns, leaving eventual release to a pool'sbackground re-probe (
isSlotLockReleased()), the recycle parks the producer on this wait so therebuild never collides with a retained flock. Rows are refused only for that window, and only while
the SF worker is genuinely wedged — a durability-failure class, the same kind of signal
SegmentRing.checkDurability()already surfaces on the row-send path — never on a transport outage,and no reconnect budget applies to the producer. The deferred-close park's interrupt policy is
code-reviewed but intentionally untested this round (the starvation wait's policy has the tests).
dropped delta baseline runs through the same
sealAndSwapBuffer/appendBlockingpath as any otherpublish, on the producer thread, bounded by
sf_append_deadline_millis(default 30 s) — stated as itsown bound, like the deferred-engine-close wait. A timeout degrades to the plain baseline drop and
closes any chunks already on the ring best-effort (
commitOrphanedDictionaryChunks, which neverthrows); if that best-effort close still leaves the group open, every subsequent
flush()andclose()re-attempts the closing commit whenever a group is open (hasDeferredMessages), so theresidual is a retried, not permanent, state.
9.x server, so no compatibility shim for 9.x server behaviour exists or is argued from.
Senderstays single-threaded. A concurrentclose()while a recycle waits is outside thedocumented contract (
Sender's own thread-safety javadoc).table(), by contract.Sender.resetSymbolDictionary(), the dictionary-caperror message, and
armIfEligible()'s javadoc all state the trigger explicitly: the recycle runs ata
table()call that finds the backlog acknowledged, and a caller that never starts another row neverrecycles. No per-row branch is added to the hot path.
during a give-back flush ejects the slot via
discardBroken, which removes it from the pool's slotset so the pool can grow back into a fresh one on demand — safe, at a capacity cost until it does. The
pooled-path recycle e2e is tracked in issue 1 million of district symbols limit #80.
Tests
13 new client suites (arming, FSN continuity, swap, memory mode, starvation, refusals, outage/orphans, catch-up skip, crash windows, healing/metrics, deferred close, slot heal/breach, reconnect fault injection — 63 tests), OSS disconnect-clear contract pin + real-client e2e (SF and memory modes) + seeded recycle×reconnect fuzz, ENT failover losslessness with recycles in flight. Verified green from a clean reactor build: client 3068/0/0/2, OSS cutlass/qwp 1397/0/0/3, ENT failover pair 3/0/0/0; the reconnect-fault counterfactuals re-run red at the final tree.
Companion PRs
🤖 Generated with Claude Code
https://claude.ai/code/session_01K2Hw6TfgX9sBB2L8oSdaWn
https://claude.ai/code/session_01NAsaMbjHq9NAENT1cJWbdg